Skip to content

Conversation

@icweaver
Copy link
Member

@icweaver icweaver commented Nov 1, 2025

Doc preview: https://juliaastro.org/Spectra.jl/previews/PR24/

Closes: #25

Currently covering representation 1 and 3 here https://specutils.readthedocs.io/en/latest/types_of_spectra.html

What's changed

  • Spectrum, EchelleSpectrum, and a new IFUSpectrum for spectral cube support are now just Spectrums, with their dimensionality specified in the type domain, i.e.,

    const SingleSpectrum = Spectrum{W, F, 1} where {W, F}
    const EchelleSpectrum = Spectrum{W, F, 2} where {W, F}
    const IFUSpectrum = Spectrum{W, F, 1, 3} where {W, F}
  • More of the array interface is implemented now so things like this will work:

    julia> spec_1D = spectrum([20, 40, 120, 160, 200], [1, 3, 6, 7, 20])
    SingleSpectrum(Int64, Int64)
      wave (5,): 20 .. 200
      flux (5,): 1 .. 20
      meta: Dict{Symbol, Any}()
    
    julia> spec_1D[2:4]
    SingleSpectrum(Int64, Int64)
      wave (3,): 40 .. 160
      flux (3,): 3 .. 7
      meta: Dict{Symbol, Any}()
    
    julia> spec_1D[2:4] |> Spectra.wave
    3-element Vector{Int64}:
      40
     120
     160
  • Merged common.jl into Spectra.jl (for my own sanity)

  • Renamed the Analysis page of the docs to Utilities and moved blackbody over to it

  • Filled in some more API documentation

To-do

  • array ops
  • enforce that wavelength arrays are ordered
  • enforce that wavelengths are the same before doing algebra on Spectrums
  • what do with meta (maybe switch to NamedTuple)
  • better (more?) show methods
  • tests
  • docs

@icweaver icweaver added the enhancement New feature or request label Nov 1, 2025
@icweaver icweaver mentioned this pull request Nov 1, 2025
@codecov
Copy link

codecov bot commented Nov 1, 2025

Codecov Report

❌ Patch coverage is 88.49558% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.01%. Comparing base (b52dd62) to head (f8dc7b9).

Files with missing lines Patch % Lines
src/spectrum_ifu.jl 60.00% 8 Missing ⚠️
src/Spectra.jl 96.15% 2 Missing ⚠️
src/spectrum_echelle.jl 90.00% 2 Missing ⚠️
src/plotting.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #24       +/-   ##
===========================================
+ Coverage   74.28%   91.01%   +16.72%     
===========================================
  Files           8        8               
  Lines         140      178       +38     
===========================================
+ Hits          104      162       +58     
+ Misses         36       16       -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@icweaver icweaver self-assigned this Nov 1, 2025
@cgarling
Copy link
Member

cgarling commented Nov 1, 2025

We did this in PhotometricFilters.jl, see here.

Is it useful for getindex to return a Spectrum object and the metadata? I would expect something like

Base.getindex(f::Spectrum, i::Int) = (wave(f)[i], flux(f)[i])

@icweaver
Copy link
Member Author

icweaver commented Nov 2, 2025

Thanks! Yea, I liked how this keeps things consistent with EchelleSpcetrum's behavior.

Actually...

This may go nowhere, but I think I want to start investigating #25 (comment) now

@icweaver icweaver added planning This is a discussion about the code and implementations. and removed enhancement New feature or request labels Nov 2, 2025
@icweaver icweaver changed the title Spectrum: added indexing Unify Spectrum and EchelleSpectrum Nov 2, 2025
@icweaver icweaver added this to the Registration milestone Nov 2, 2025
@icweaver icweaver mentioned this pull request Nov 3, 2025
3 tasks
@icweaver
Copy link
Member Author

icweaver commented Nov 3, 2025

Ok, I think this might be going somewhere. Updated the parent comment with a summary

@icweaver icweaver marked this pull request as ready for review November 3, 2025 05:35
@cgarling
Copy link
Member

cgarling commented Nov 3, 2025

If you want to cover specutils' case 2 (multi-dimensional flux, but same spectral axis along each) you may need to add a type parameter for the dimensionality of the wavelength array as well. I think case 2 is for IFU data (??) so maybe

mutable struct Spectrum{W<:Number, F<:Number, N, M} <: AbstractSpectrum{W, F}
    wave::AbstractArray{W, N}
    flux::AbstractArray{F, M}
    meta::Dict{Symbol,Any}
end

const SingleSpectrum = Spectrum{W, F, 1, 1} where {W, F}
const IFUSpectrum = Spectrum{W, F, 1, 2} where {W, F}
const EchelleSpectrum = Spectrum{W, F, 2, 2} where {W, F}

We can also worry about this later if you want

@icweaver
Copy link
Member Author

icweaver commented Nov 3, 2025

oh I like this a lot. Added some methods to start playing with this. There's something really satisfying about being able to do, say:

julia> spec_IFU = spectrum([20, 40, 120, 160, 200], rand(10, 5))
IFUSpectrum(Int64, Float64)
  # orders: 10
  wave: (20, 200)
  meta: Dict{Symbol, Any}()

julia> spec_IFU[3, begin:4]
SingleSpectrum(Int64, Float64)
  wave: (20, 200)
  flux: (0.12659667784419948, 0.3942549580490494)
  meta: Dict{Symbol, Any}(:Order => 3)

to get the first 4 flux measurements from the 3rd order of an IFUSpectrum. I imagine this could be opened up a bit more to allow for more uniform indexing across the other spectrum types. I don't do space rainbow science much these days, so does this sound like something that would be useful to have, or is this trying to do too much?

@cgarling
Copy link
Member

cgarling commented Nov 3, 2025

Actually I think an IFU with a uniform wavelength (dispersion) sampling would be a 3-D cube (2 spatial axes across the IFU field, one wavelength axis) so M = 3, see here for example.

I think it's a good idea, not sure what the best API is. I would probably put the wavelength axis first in the flux matrix because the most common operation is probably selecting a 1-D spectrum from the cube based on pixel.

julia> wave, flux = [20, 40, 120, 160, 200], rand(5, 10, 10);

julia> spec_ifu = spectrum(wave, flux);

julia> spec_ifu[:, 1, 1] # Selects full 1-D spectrum from spatial pixel with index (1, 1)

My intuition is that a scalar wavelength index would return a basic array (i.e, an image of the flux at a fixed wavelength).

julia> spec_IFU[3, begin:4, begin:4] isa Matrix{Float64}
true

julia> spec_IFU[3, begin:4, begin:4] == flux[3, begin:4, begin:4]
true

I think returning a spectrum type only makes sense if the index is a range for the wavelength. I'm really not sure though, I don't really work on this either

@icweaver
Copy link
Member Author

icweaver commented Nov 4, 2025

Oh cool! Thanks for the reference, I've never worked with IFUs before, so this was a really interesting read. I think "spaxel" is my new favorite word 😂. I see that I was mixing concepts between echelles and ifus when I was talking about orders, sorry about that. The split to 3D really helped clarify things for me. How does this new design look? I also cleaned up the show methods a bit (still need to update tests once this settles though)

julia> using Spectra

julia> wave, flux = [20, 40, 120, 160, 200], rand(5, 10, 6);

julia> spec_ifu = spectrum(wave, flux)
IFUSpectrum(Int64, Float64)
  wave ((5,)): 20 .. 200
  flux ((5, 10, 6)): 0.6211135421955702 .. 0.4976591674645773
  meta: Dict{Symbol, Any}()

julia> spec_ifu[:, 1, 1]
SingleSpectrum(Int64, Float64)
  wave ((5,)): 20 .. 200
  flux ((5,)): 0.6211135421955702 .. 0.9046136130205523
  meta: Dict{Symbol, Any}()

julia> spec_ifu[:, begin:4, begin:3]
IFUSpectrum(Int64, Float64)
  wave ((5,)): 20 .. 200
  flux ((5, 4, 3)): 0.6211135421955702 .. 0.8211549470853685
  meta: Dict{Symbol, Any}()

julia> spec_ifu[3, begin:4, begin:3]
4×3 Matrix{Float64}:
 0.184341  0.942616   0.546656
 0.206153  0.754342   0.120533
 0.186781  0.0043856  0.0914326
 0.601994  0.352066   0.82047

julia> spec_ifu[3, begin:4, begin:3] == flux[3, begin:4, begin:3]
true

@icweaver icweaver changed the title Unify Spectrum and EchelleSpectrum Unify XSpectrum types Nov 4, 2025
@icweaver icweaver linked an issue Nov 4, 2025 that may be closed by this pull request
5 tasks
@icweaver
Copy link
Member Author

icweaver commented Nov 7, 2025

Ok, I think the merge of the different Spectrum array types is starting to shape up nicely (no pun intended). I've added some various usage examples to the docs for review.

The biggest, and maybe most controversial, change is switching from row major to column major representation. While this differs from specutils' representation, I like that it unifies dimension and wavelength checks across all Spectrum types (no separate asserts or per-Spectrum methods needed anymore). Also, something about the non-alphabetical ordering of Spectrum{W<:Number, F<:Number, N, M} from before made my eye twitch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

planning This is a discussion about the code and implementations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

More array support? Register

3 participants